fix: record-ring orientation, ingest verdict pairing, CORS self-trust, and smoke alerts that clear - #85
Merged
Conversation
The August 2026 cutover proved that a *configured* CORS allowlist can be worse than none: REGENGINE_CORS_ORIGINS was set — to the previous service's URL, via a Railway reference variable — so the new service rejected every browser request from its own domain for three days, and because auth_middleware gates state-changing requests on the same list, writes failed too (#80, #81). Append the platform-issued origin (https://$RAILWAY_PUBLIC_DOMAIN) to whatever cors_origins_from_env() resolves. A union rather than a fallback, deliberately: the incident had the variable set-but-stale, so a fallback that only applies when the variable is missing would have changed nothing. Trusting the platform domain widens nothing beyond the service's own canonical origin, and anyone who can forge that variable already controls the deployment. A malformed platform value degrades to "no extra origin" instead of raising — this path runs while the ASGI app is constructed, and a platform-injected string must never be able to crash startup. Explicit origins for third-party dashboard hosts still require configuration; the cutover checklist keeps that step and notes the self-trust behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ykFQkKR1XmCtSkDRCT4sT
`_records` is newest-first, and that is a contract rather than a convention: `add_many` uses `appendleft`, `recent()` reads a left-slice, and `maxlen` eviction drops from the right. Three methods rebuilt the ring and all three did it differently — only one was correct. `update_many` rebuilt straight from `_all_records()`, which sorts *ascending* by `sequence_no`, leaving the deque oldest-first. `recent()` then served the oldest events, and the next `appendleft` evicted from what had become the newest end — writes deleting the data just written. One delivery retry triggers it. `replace_all` had a different bug in the same family. It reversed before truncating, and `deque(iterable, maxlen=n)` keeps the *last* n items, so over capacity it retained the n oldest records and discarded everything recent. Truncation has to happen while the sequence is still oldest-first. All three now route through `_set_records`, which truncates and then reverses, so the call sites cannot drift apart again. Why this survived a test suite that already exercised `update_many` twice: those tests read back through a *fresh* `EventStore`, and `_load_from_disk` re-sorts and re-reverses on the way in — silently repairing the state before anything was asserted about it. The other reader reset with `batch_size: 1`, where an inverted list is identical to a correct one. The three tests added here all read back through the same instance, and each was verified to fail against the original code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ykFQkKR1XmCtSkDRCT4sT
RegEngine does not answer in request order. Its webhook route appends replay-window and KDE rejections first, then rules-enforcement rejections, then every acceptance, so the response `events` list is partitioned rejected-first. Inflow Lab zipped that list against the request by array index, which misattributes as soon as a rejection follows an acceptance: a valid lot is stored as `failed` carrying another lot's validation errors, and the `event_id`, `sha256_hash` and `chain_hash` of a genuinely accepted event are copied onto the wrong record. Those hashes are the evidence. `(traceability_lot_code, cte_type)` is already on the wire on both sides and is the join key. It can legitimately repeat inside one batch — the same lot and CTE at two timestamps — so responses are held in a per-key queue and consumed in order rather than collapsed into a dict. A key with no response left reads as `None`, meaning no verdict. Positional fallback is deliberately gone: there was always *some* answer before, which is exactly why nothing ever looked wrong. Four call sites had this, not two. Beyond `step()` and the CSV import, the demo-fixture load and the delivery-retry path zipped by index too — and the retry path is the worst of them, since a retry that succeeds for one record could mark a different record posted. Why the existing suite is blind to it: every other test delivers through `app/mock_service.py`, which appends accepted and rejected in a single pass, so the mock's response *is* in request order. The new tests supply a genuinely partitioned response. The end-to-end one was verified to fail against the original code — it stored the rejected lot as `posted`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ykFQkKR1XmCtSkDRCT4sT
The smoke alerts only ever fired. Nothing closed them, and the issue body said so out loud — "Close it once the smoke passes again" — making a human the entire recovery mechanism. #80 and #81 both stayed open for days after the underlying failure was fixed. The second-order effect is the more damaging half. The tracker lookup is scoped to `state: 'open'`, so a stale tracker from a resolved incident silently absorbs the *next* genuine outage as one more comment on an aged thread. The strongest signal GitHub offers — a brand-new issue — was spent exactly once, on the first failure ever. `smoke-failure-issue.yml` gains a `resolved` input and a close path; both smoke workflows now call it on success as well as on failure. The close path closes every match rather than the first, since duplicates predating this edge would otherwise keep the stale-tracker problem alive, and it skips pull requests the same way the open path now does. Verified by running both scripts against a mock GitHub API: first failure opens an issue, a second comments without duplicating, recovery comments and closes, and — the point of the change — the next outage after a recovery opens a *new* issue rather than commenting on the closed one. Recovery with nothing open is a no-op, and a PR carrying the marker in its body is never closed as a tracker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ykFQkKR1XmCtSkDRCT4sT
…says what cannot
The cutover checklist in DEPLOYMENT_PROFILES.md is followed by hand, which is
how the August 2026 attempt went wrong: the new service referenced the old
one's variables, and both nightly smokes stayed red for three days with the
failure attributed to the wrong cause.
`scripts/cutover_preflight.sh` mechanises the parts observable from outside —
every read-only path in the dashboard's proxy contract answering alike on both
services, the new service reporting GitHub-injected build identity rather than
a stale REGENGINE_BUILD_SHA, and the new service trusting its own origin. It
compares against the old service rather than asserting absolutes, because
matching whatever currently serves production is the standard that matters.
Read-only by construction: the demo is shared, and the POST routes the
dashboard proxies (simulate start/stop/reset, fixture load) mutate its state.
The more important half is what it refuses to claim. The Basic-auth credentials
live as secrets on two platforms, so from outside a service with the wrong
credentials is indistinguishable from one with the right ones — both answer 401
to an unauthenticated probe. Rather than let a green run imply a clean bill of
health, the success path names that gap: Vercel's INFLOW_LAB_BASIC_AUTH_* must
equal the new service's REGENGINE_BASIC_AUTH_* as concrete values, or every
proxied call 401s the moment the URL is flipped.
It also directs the post-flip check at /api/simulate/status rather than
/api/healthz, since the proxy answers 200 with {"offline":true} when the
backend is unreachable — the same degraded-200 that made an earlier uptime
probe unable to see an outage.
Verified against the live pair. Passing run: all eight proxied read paths
match, new service reports a698ceb via RAILWAY_GIT_COMMIT_SHA, both services
trust their own origin. Both failure paths exercised too — pointing "new" at
the non-GitHub service trips the build-identity check, and an unreachable host
trips the contract and CORS checks. Exit status is non-zero in both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ykFQkKR1XmCtSkDRCT4sT
PetrefiedThunder
marked this pull request as ready for review
August 19, 2026 07:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four fixes plus one tool. The CORS change is the original scope; the rest came from a systematic silent-failure audit run afterwards. Two of them corrupt data today.
1.
EventStorerecord ring inverted after any delivery retry — LIVE_recordsis newest-first, and that is a contract, not a convention:add_manyusesappendleft,recent()reads a left-slice, andmaxleneviction drops from the right.update_manyrebuilt from_all_records(), which sorts ascending bysequence_no, leaving the deque oldest-first.recent()then served the oldest events, and the nextappendleftevicted from what had become the newest end — writes deleting the data just written. One delivery retry, a single click, triggers it.replace_allhad a different bug in the same family, and it was the one cited elsewhere as the correct reference implementation. It reversed before truncating, anddeque(iterable, maxlen=n)keeps the last n items, so over capacity it retained the n oldest records and discarded everything recent.All three rebuild sites now route through
_set_records, which truncates and then reverses, so they cannot drift apart again.Why a suite that already exercised
update_manytwice never saw it: those tests read back through a freshEventStore, and_load_from_diskre-sorts and re-reverses on the way in — silently repairing the state before anything was asserted about it. The other reader reset withbatch_size: 1, where an inverted list is identical to a correct one. The three tests added here all read back through the same instance, and each was verified to fail against the original code.2. Ingest verdicts matched by array position — LIVE
RegEngine does not answer in request order. Its webhook route appends replay-window and KDE rejections first, then rules-enforcement rejections, then every acceptance, so the response
eventslist is partitioned rejected-first.Zipping it against the request by index misattributes as soon as a rejection follows an acceptance: a valid lot is stored as
failedcarrying another lot's validation errors, and theevent_id,sha256_hashandchain_hashof a genuinely accepted event are copied onto the wrong record. Those hashes are the evidence.(traceability_lot_code, cte_type)is already on the wire on both sides and is the join key. It can legitimately repeat inside one batch — the same lot and CTE at two timestamps — so responses are held in a per-key queue and consumed in order rather than collapsed into a dict. A key with no response left reads asNone, meaning no verdict. Positional fallback is deliberately gone: there was always some answer before, which is exactly why nothing ever looked wrong.Four call sites had this, not two. Beyond
step()and the CSV import, the demo-fixture load and the delivery-retry path zipped by index too — and the retry path is the worst, since a retry that succeeds for one record could mark a different record posted.Why the existing suite is blind to it: every other test delivers through
app/mock_service.py, which appends accepted and rejected in a single pass, so the mock's response is in request order. The new tests supply a genuinely partitioned response; the end-to-end one was verified to fail against the original code, storing the rejected lot asposted.3. CORS: trust the service's own platform origin
Makes the service trust
https://$RAILWAY_PUBLIC_DOMAINin addition to whateverREGENGINE_CORS_ORIGINSresolves to. This is the code half of the August cutover lesson; the docs half landed in #84.A union, not a fallback. In the incident the variable was set — to the previous service's URL, via a Railway reference variable — so a fallback that only applies when the variable is missing would have changed nothing. The same list also gates state-changing requests in
auth_middleware, so browser writes failed too, and both nightly smokes stayed red for three days (#80, #81).Safe: the appended origin is only the service's own canonical domain, injected by Railway. Anyone who can forge
RAILWAY_PUBLIC_DOMAINalready controls the deployment. Wildcards remain rejected, and explicit origins for any other host still require configuration. A malformed platform value degrades to "no extra origin" rather than raising — this runs while the ASGI app is being constructed, and a platform-injected string must never crash startup.4. Smoke alerts that clear
The alerts only ever fired. Nothing closed them, and the issue body said so out loud — "Close it once the smoke passes again" — making a human the entire recovery mechanism. #80 and #81 both stayed open for days after the underlying failure was fixed.
The second-order effect is the more damaging half: the tracker lookup is scoped to
state: 'open', so a stale tracker from a resolved incident silently absorbs the next genuine outage as one more comment on an aged thread. The strongest signal GitHub offers — a brand-new issue — was spent exactly once, on the first failure ever.smoke-failure-issue.ymlgains aresolvedinput and a close path; both smoke workflows now call it on success as well as failure. It closes every match rather than the first, since duplicates predating this edge would keep the problem alive, and it skips pull requests the same way the open path now does.Verified by running both scripts against a mock GitHub API: first failure opens an issue, a second comments without duplicating, recovery comments and closes, and — the point of the change — the next outage after a recovery opens a new issue rather than commenting on the closed one. Recovery with nothing open is a no-op, and a PR carrying the marker in its body is never closed as a tracker.
5.
scripts/cutover_preflight.shMechanises what the cutover checklist can verify from outside: every read-only path in the dashboard's proxy contract answering alike on both services, the new service reporting GitHub-injected build identity rather than a stale
REGENGINE_BUILD_SHA, and the new service trusting its own origin. Read-only by construction — the demo is shared, and the POST routes the dashboard proxies mutate its state.The more important half is what it refuses to claim. The Basic-auth credentials live as secrets on two platforms, so from outside a service with the wrong credentials is indistinguishable from one with the right ones — both answer 401. Rather than let a green run imply a clean bill of health, the success path names that gap and directs the post-flip check at
/api/simulate/statusrather than/api/healthz, since the proxy answers 200 with{"offline":true}when the backend is unreachable.Test Plan
posted)pytestandbrowser-smokegreen on0c428a5